refactor(git-read): use shared search seams - #368
seekskyworld wants to merge 2 commits into
Conversation
tt-a1i
left a comment
tt-a1i
left a comment
There was a problem hiding this comment.
Exact-head review: a9d7ec4
[P1] This moves the import name but does not remove the cross-extension private dependency required by Issue #73. extensions/shared/search-output.ts and search-process.ts are only re-export shims whose implementations still come from ../file-search/src/output.ts and ../file-search/src/process.ts. The shared layer therefore depends on a feature extension private src tree; refactoring or packaging file-search can still break git-read through the new alias.
Issue #73 acceptance requires extensions to stop importing another extension private implementation, and the PR claims a stable shared seam. Please move the reusable bounded-output/process implementation into extensions/shared (or define a genuinely owner-neutral shared module), then have both file-search and git-read depend on it. Add an import-boundary guard so a future shared-to-feature dependency fails CI.
Verification boundary: exact-head Standards + Spec static review; no runtime behavior change is claimed, so the blocker is the unresolved dependency direction itself.
Move bounded search output/process implementations into extensions/shared so file-search and git-read depend on an owner-neutral module. Add an import-boundary regression so shared cannot depend on feature extensions.
a9d7ec4 to
d0572d0
Compare
|
Rebased onto current
|
tt-a1i
left a comment
tt-a1i
left a comment
There was a problem hiding this comment.
审查提交：d0572d0758e9c313f1ed1c893e85f5fa473f90db。
本轮未发现新的阻断问题。以下结论仅覆盖所列验证范围，不是合并批准。
验证范围
Shared implementation is now physically moved, not re-exported from feature internals: old architecture finding resolved. Full rename-aware diff reviewed. 14 Node tests and 30 file-search Vitest tests pass.
限制与后续
Needs current-main integration and refreshed final-head checks. No native Windows runtime run locally.
tt-a1i
left a comment
tt-a1i
left a comment
There was a problem hiding this comment.
Reviewed the complete rename-aware diff again. Shared bounded capture implementation is physically moved to shared modules; both consumers and tests use the new imports. No blocking issue found. Required CI passed on this head; refreshed current-main checks are required before merge. Exact reviewed head: d0572d0.
tt-a1i
left a comment
tt-a1i
left a comment
There was a problem hiding this comment.
Integration head c6cd37a fails all three Node CI jobs. [P2] Make the new import-boundary gate compatible with current main (tests/extensions/shared/import-boundary.test.ts:7-21). The regex scans every shared source and rejects the existing type-only import from ../subagents/src/domain.ts in web-observer-registry.ts. Node 24 job 108427629093 reports actual offenders [web-observer-registry.ts] versus expected []. The production search-module move remains sound, but this new required test prevents the integrated branch from passing bun run test. Define the intended type/runtime boundary and accommodate or migrate the existing shared type contract, then rerun final-head CI. This is a deterministic integration failure, not an infrastructure retry.
This branch has not been deployed
2 participants
Problem
Implements the cross-extension import slice of #73.
git-readdirectly imported privatefile-search/srcmodules, coupling extension internals.Value
A named shared seam makes the dependency direction explicit and gives future refactors one stable import boundary without changing command, output, or process semantics.
Approach
Add shared re-export seams for bounded search output and process execution, then update
git-readto consume only those shared modules. The underlying implementation and behavior remain unchanged.Validation
npx tsc --noEmitgit diff --checkImpact